feat(iOS): early TurboModule discovery #53277
Conversation
meta-cla
bot
added
the
CLA Signed
tjzel
force-pushed
the
@tjzel/ios-turbo-module-discovery
branch
from
deb7658 to
b1984c7
Compare
facebook-github-bot
added
the
Shared with Meta
There was a problem hiding this comment.
Thanks for the PR. I left a few comments that has to be addressed.
This feature might also create some performance issues, delaying the app startup.
It would be great if you can wrap the _discoverModules in a featureFlag check, so we can disable it if we have a regression in the startup performance.
Feature flags for React Native are documented here
|
|
||
| @protocol RCTTurboModuleManagerDelegate <NSObject> | ||
|
|
||
| - (nonnull NSArray<NSString *> *)getModuleNames; |
There was a problem hiding this comment.
this is a breaking change. every implementer of the delegate will have to implement this.
It is better to move this to be optional and to handle the optionality.
Also, the documentation is missing.
There was a problem hiding this comment.
I didn't know there is an application for multiple delegate implementers. I'll make it optional as you suggest and add relevant documentation.
| #pragma mark - Private Methods | ||
|
|
||
| - (void)_discoverModules{ | ||
| NSArray<NSString *> *moduleNames = [_delegate getModuleNames]; |
There was a problem hiding this comment.
please, make the method in the delegate @optional, and handle this here like:
if ([_delegate respondsToSelector:@selector(getModuleNames)]) {
...
}
| #pragma mark - RCTTurboModuleManagerDelegate | ||
|
|
||
| - (nonnull NSArray<NSString *> *)getModuleNames{ | ||
| return [_delegate getModuleNames]; |
There was a problem hiding this comment.
we should check whether the delegate implements this method or not.
| } | ||
|
|
||
| - (nonnull NSArray<NSString *> *)getModuleNames{ | ||
| return [dependencyProvider moduleNames]; |
There was a problem hiding this comment.
we should check whether the dependencyProvider implement this method.
People could, in principle, create custom dependency providers
|
|
Thanks for the review! I tested the changes flipping the feature flag on and off and it worked as expected. |
|
@tjzel, could you elaborate on the motivation behind this change? Why do we need the module holders initialized early? |
|
@RSNara I talked about it in this comment #53282 (comment), early module holder initialization was for the purpose of finding modules conforming to a certain protocol. |
|
Closing in favor of #53928 |
5 participants
Summary:
At the moment user-made TurboModules are discovered during the evaluation of the script - conversely to Android, where it happens before this evaluation. Because of that it's impossible to instantiate a module that we don't know the name of pre-bundle.
To implement this I used
modulesProvidercodegen config field to get the names of the TurboModules and initialize their holders early.Changelog:
[IOS] [ADDED] - Added ability to discover TurboModules pre-bundle.
Test Plan: